Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new option dropdown below menu as described in #278 #279

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

hlvs-apps
Copy link
Contributor

See #278

@AhmedLSayed9
Copy link
Owner

I'm developing a font chooser menu, and don't want the chosen font displayed in the drop-down button to be hidden so the user can compare the current selected font with the other ones.

I'm not getting what is the use case exactly.
Can you elaborate more by a real life use-case example? some illustrations might be helpful.

Anyway the current change breaks the expected behavior:

  final List<String> items = [
    'Item1',
    'Item2',
    'Item3',
    'Item4',
  ];
  final valueListenable = ValueNotifier<String?>(null);

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: DropdownButtonHideUnderline(
          child: Column(
            mainAxisAlignment: MainAxisAlignment.end,
            children: [
              DropdownButton2<String>(
                isExpanded: true,
                dropdownOnlyBelowButton: true,
                hint: Text(
                  'Select Item',
                  style: TextStyle(
                    fontSize: 14,
                    color: Theme.of(context).hintColor,
                  ),
                ),
                items: items
                    .map((String item) => DropdownItem<String>(
                          value: item,
                          height: 40,
                          child: Text(
                            item,
                            style: const TextStyle(
                              fontSize: 14,
                            ),
                          ),
                        ))
                    .toList(),
                valueListenable: valueListenable,
                onChanged: (String? value) {
                  valueListenable.value = value;
                },
                buttonStyleData: const ButtonStyleData(
                  padding: EdgeInsets.symmetric(horizontal: 16),
                  height: 40,
                  width: 140,
                ),
              ),
              const SizedBox(height: 100),
            ],
          ),
        ),
      ),
    );
  }
Screenshot 2024-05-28 at 11 35 02 PM

@AhmedLSayed9 AhmedLSayed9 added the question Further information is requested label May 28, 2024
@hlvs-apps
Copy link
Contributor Author

For example, this feature could be used in a font chooser plugin:
the dropdown menu displays a huge list of all supported fonts to let the user select the right font:
Font Chooser Menu

For choosing the right font, it is essential to compare the new choices with the old one. However, with the current dropdown menu the dropdown menu button is hidden behind the dopown menu. With the new proposed option enabled, this is not the case anymore:
New Option Enabled

Also, while searching a huge dropdown list with no or few hits, without the new proposed option, something like this could occur:
Old approach empty searchresponse

And with the option enabled:
New approach empty searchresponse

@hlvs-apps
Copy link
Contributor Author

I fixed the bug that broke the expected behaviour.

@AhmedLSayed9
Copy link
Owner

Tbh, I'm not sure if limiting the menu to be from the bottom of the button to the end of the screen is a good idea and I think it will lead to bad ux.

What if the button is at the end of the screen? or, what if the screen is scrollable and the button happens to be at the bottom?

Also, Why not just set the maximum height of the dropdown menu? i.e, in this screenshot:
Old approach empty searchresponse
You can just set proper max height for the menu depending on the button's position or your preferences.

imo, this update will complicate things for nothing while also may produce bad ux as explained above.

@AhmedLSayed9
Copy link
Owner

I appreciate your effort anyway :)

Can you share the font chooser example you used? I'll explain more about my points above.

@hlvs-apps
Copy link
Contributor Author

hlvs-apps commented May 29, 2024

Here is a simplified version of the example shown above, without the loading algorithms that currently depend on a own server:

import 'package:dropdown_button2/dropdown_button2.dart';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
//import 'package:flutterfontchooser/flutterfontchooser.dart';


void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Test',
      theme: ThemeData(
        colorScheme: ColorScheme.fromSeed(seedColor: Colors.deepPurple),
        useMaterial3: true,
      ),
      home: const MyHomePage(),
    );
  }
}

class MyHomePage extends StatelessWidget {
  const MyHomePage({super.key});

  @override
  Widget build(BuildContext context) {
    return const Scaffold(
      body: Center(
        child: DropdownButtonHideUnderline(
          child: Column(
            mainAxisAlignment: MainAxisAlignment.end,
            children: [
              Text("Width: ..."),
              Text("Height: 500px"),
              Text("..."),
              Row(
                mainAxisAlignment: MainAxisAlignment.spaceEvenly,
                children: [
                  Text("Bold"),
                  Text("Italic"),
                ],
              ),
              FontChooserDropdownMenuWithoutServerAccessAndNoFonts(),
              //FontChooserDropdownMenu(),
              SizedBox(height: 400),
            ],
          ),
        ),
      ),
    );
  }
}

//a dropdown widget that allows the user to choose a font, and displays the font name in the dropdown.
//the fonts get dynamically loaded from the server
class FontChooserDropdownMenuWithoutServerAccessAndNoFonts extends StatefulWidget {
  //const FontChooserDropdownMenu({super.key, super.controller});
  const FontChooserDropdownMenuWithoutServerAccessAndNoFonts({super.key});

  @override
  State<FontChooserDropdownMenuWithoutServerAccessAndNoFonts> createState() =>
      _FontChooserDropdownMenuWithoutServerAccessAndNoFontsState();
}

class _FontChooserDropdownMenuWithoutServerAccessAndNoFontsState
    extends State<FontChooserDropdownMenuWithoutServerAccessAndNoFonts> {
  //final Map<SearchResponseEntryComparable, DropdownItem<SearchResponseEntryComparable>>
  final Map<String, DropdownItem<String>>
  _items = {};


  void _loadItems() {
    /*Set<SearchResponseEntryComparable> oldItems = _items.keys.toSet();
    bool changed = false;
    for (var value in controller.allFontsIfLoaded) {
      SearchResponseEntryComparable comparable =
      SearchResponseEntryComparable(value);
      if (!oldItems.contains(comparable)) {
        changed = true;
        _items[comparable] = DropdownItem<SearchResponseEntryComparable>(
          value: comparable,
          child: DropdownRowWidget(
            key: Key("${value.family}-dropdown-row-widget"),
            selectedFont: selectedFont,
            thisFont: value,
          ),
        );
      }
      oldItems.remove(comparable);
    }
    for (var item in oldItems) {
      _items.remove(item);
    }
    if (changed && mounted) {
        setState(() {});
    }*/
    for(int i=0; i<1500;i++){
      _items["Font $i"] = DropdownItem<String>(
        value: "Font $i",
        child: DropdownRowWidget(
          key: Key("Font $i-dropdown-row-widget"),
          selectedFont: selectedFont,
          thisFont: "Font $i",
        ),
      );
    }
    setState(() {});
  }

  @override
  void initState() {
    super.initState();
    ServicesBinding.instance.keyboard.addHandler(_onKey);
    Future.microtask(_loadItems);
  }

  /*@override
  void onController() {
    var s = searchResponseEntryComparableFromSearchResponseEntry(
        controller.selectedFont?.searchResponseEntry);
    if (s != selectedFont.value) {
      selectedFont.value = s;
    }
    _loadItems();
  }*/

  @override
  void dispose() {
    ServicesBinding.instance.keyboard.removeHandler(_onKey);
    super.dispose();
  }

  //final ValueNotifier<SearchResponseEntryComparable?> selectedFont =
  final ValueNotifier<String?> selectedFont =
  ValueNotifier(null);

  TextEditingController searchController = TextEditingController();

  FocusNode searchFocusNode = FocusNode();

  bool _onKey(KeyEvent event) {
    if (_menuOpen && !searchFocusNode.hasFocus && event is KeyDownEvent) {
      String? key = event.character;
      if (key != null &&
          event.logicalKey != LogicalKeyboardKey.enter &&
          event.logicalKey != LogicalKeyboardKey.space &&
          event.logicalKey != LogicalKeyboardKey.tab) {
        if (event.logicalKey == LogicalKeyboardKey.backspace) {
          if (searchController.text.isNotEmpty) {
            searchController.text = "";
            searchFocusNode.requestFocus();
            return true;
          }
          searchFocusNode.requestFocus();
          return false;
        }
        searchController.text += key;
        searchFocusNode.requestFocus();
        return true;
      }
    }
    return false;
  }

  bool _menuOpen = false;


  @override
  Widget build(BuildContext context) {
    //return DropdownButton2<SearchResponseEntryComparable>(
    return DropdownButton2<String>(
      valueListenable: selectedFont,
      barrierCoversButton: false,
      dropdownOnlyBelowButton: true,
      onMenuStateChange: (open) {
        _menuOpen = open;
      },
      buttonStyleData: const ButtonStyleData(
        width: 260,
        height: 50,
      ),
      dropdownSearchData: DropdownSearchData(
        searchController: searchController,
        searchBarWidgetHeight: 50,
        searchBarWidget: Container(
          height: 50,
          padding: const EdgeInsets.only(
            top: 8,
            bottom: 4,
            right: 8,
            left: 8,
          ),
          child: TextFormField(
            maxLines: 1,
            controller: searchController,
            focusNode: searchFocusNode,
            decoration: InputDecoration(
              isDense: true,
              contentPadding: const EdgeInsets.symmetric(
                horizontal: 10,
                vertical: 8,
              ),
              hintText: 'Search for a font...',
              hintStyle: const TextStyle(fontSize: 12),
              border: OutlineInputBorder(
                borderRadius: BorderRadius.circular(8),
              ),
            ),
          ),
        ),
        noResultsWidget: const Padding(
          padding: EdgeInsets.all(8),
          child: Text('No Font Found!'),
        ),
        searchMatchFn:
            //(DropdownItem<SearchResponseEntryComparable> entry, String search) {
            (DropdownItem<String> entry, String search) {
          //return entry.value?.family
           return entry.value?.toLowerCase()
              .contains(search.toLowerCase()) ??
              false;
        },
      ),
      iconStyleData: const IconStyleData(
        icon: Icon(
          Icons.arrow_drop_down,
          color: Colors.black45,
        ),
      ),
      dropdownStyleData: DropdownStyleData(
        decoration: BoxDecoration(
          borderRadius: BorderRadius.circular(15),
        ),
      ),
      menuItemStyleData: const MenuItemStyleData(
        padding: EdgeInsets.symmetric(horizontal: 16),
      ),
      hint: const Text(
        'Select a font',
        style: TextStyle(fontSize: 14),
      ),
      /*onChanged: (SearchResponseEntryComparable? newValue) {
        selectedFont.value = newValue;
        if (newValue == null) {
          controller.selectedFont = null;
          return;
        }
        Future.microtask(() async {
          controller.selectedFont =
          await LoadedFont.fromSearchResponseEntry(newValue.entry);
        });
      },*/
      onChanged: (String? newValue) {
        selectedFont.value = newValue;
      },
      items: _items.values.toList(),
    );
  }
}

class DropdownRowWidget extends StatefulWidget {
  //final SearchResponseEntry thisFont;
  final String thisFont;
  //final ValueNotifier<SearchResponseEntryComparable?> selectedFont;
  final ValueNotifier<String?> selectedFont;
  final double width;

  const DropdownRowWidget(
      {super.key,
        required this.thisFont,
        this.width = 200,
        required this.selectedFont});

  @override
  State<DropdownRowWidget> createState() => _DropdownRowWidgetState();
}

class _DropdownRowWidgetState extends State<DropdownRowWidget> {
  bool _isFontLoaded = false;
  bool _isLoading = false;

  @override
  Widget build(BuildContext context) {
    return Row(
      mainAxisAlignment: MainAxisAlignment.spaceBetween,
      children: [
        SizedBox(
          width: widget.width,
          child: Text(widget.thisFont,
              /*style: Theme.of(context)
                  .textTheme
                  .apply(fontFamily: widget.thisFont.menuFontFamily)
                  .bodyMedium*/),
        ),
        if (!_isFontLoaded && !_isLoading)
          const Icon(
            Icons.cloud_download_outlined,
            color: Colors.grey,
            size: 24,
          ),
        /*if (!_isFontLoaded && _isLoading)
          LoadingAnimationWidget.dotsTriangle(
            color: Colors.grey,
            size: 24,
          ),*/
      ],
    );
  }

  void _check() {
    if(!mounted){
      return;
    }
    bool s = false;
    //bool i = isFontLoaded(widget.thisFont);
    bool i =false;
    if (i != _isFontLoaded) {
      _isFontLoaded = i;
      s = true;
    }
    //i = isLoadingFont(widget.thisFont);
    i= false;
    if (i != _isLoading) {
      _isLoading = i;
      s = true;
    }
    if (s) {
      setState(() {});
    }
  }

  @override
  void initState() {
    super.initState();
    _check();
    //FontLoaderNotifier.instance.addListener(_check);
    widget.selectedFont.addListener(_check);
  }

  @override
  void didUpdateWidget(DropdownRowWidget oldWidget) {
    super.didUpdateWidget(oldWidget);
    _check();
  }

  @override
  void dispose() {
    //FontLoaderNotifier.instance.removeListener(_check);
    super.dispose();
  }
}

@hlvs-apps
Copy link
Contributor Author

I tried achieving the behaviour of my option via the max-height property, but for example opening the virtual keyboard on mobile devices or resizing the window on desktop changes the available max height, but setting the max height only gets applied each time the menu is reopened. (or is there a way to apply it directly without reopening the menu?)

@AhmedLSayed9
Copy link
Owner

AhmedLSayed9 commented May 29, 2024

I tried achieving the behaviour of my option via the max-height property, but for example opening the virtual keyboard on mobile devices or resizing the window on desktop changes the available max height, but setting the max height only gets applied each time the menu is reopened. (or is there a way to apply it directly without reopening the menu?)

The specified max height shouldn't change when you resize the window, i.e if you set 300 as max height, menu will always be 300 in height unless there isn't enough space.

@hlvs-apps
Copy link
Contributor Author

I tried achieving the behaviour of my option via the max-height property, but for example opening the virtual keyboard on mobile devices or resizing the window on desktop changes the available max height, but setting the max height only gets applied each time the menu is reopened. (or is there a way to apply it directly without reopening the menu?)

The specified max height shouldn't change when you resize the window, i.e if you set 300 as max height, menu will always be 300 in height unless there isn't enough space.

Exactly. If I understand correctly, your suggestion to always display the dropdown menu under the button is to set the max height property based on the available space so that the menu is displayed under the button. But if the available space and therefore the calculated max-height changes, e.g. because the windows is resized, the changes made to the max height property only take effect after the dropdown menu is reopened, to late to have any practical use:
max_height_approach

while with the new option:
new_option_approach

code:

import 'package:dropdown_button2/dropdown_button2.dart';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
//import 'package:flutterfontchooser/flutterfontchooser.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Test',
      theme: ThemeData(
        colorScheme: ColorScheme.fromSeed(seedColor: Colors.deepPurple),
        useMaterial3: true,
      ),
      home: const MyHomePage(),
    );
  }
}

class MyHomePage extends StatelessWidget {
  const MyHomePage({super.key});

  @override
  Widget build(BuildContext context) {
    return const Scaffold(
      body: Center(
        child: DropdownButtonHideUnderline(
          child: Column(
            mainAxisAlignment: MainAxisAlignment.start,
            children: [
              Text("Width: ..."),
              Text("Height: 500px"),
              Text("..."),
              Row(
                mainAxisAlignment: MainAxisAlignment.spaceEvenly,
                children: [
                  Text("Bold"),
                  Text("Italic"),
                ],
              ),
              FontChooserDropdownMenuWithoutServerAccessAndNoFonts(),
              //FontChooserDropdownMenu(),
              //SizedBox(height: 200),
            ],
          ),
        ),
      ),
    );
  }
}

//a dropdown widget that allows the user to choose a font, and displays the font name in the dropdown.
//the fonts get dynamically loaded from the server
class FontChooserDropdownMenuWithoutServerAccessAndNoFonts
    extends StatefulWidget {
  //const FontChooserDropdownMenu({super.key, super.controller});
  const FontChooserDropdownMenuWithoutServerAccessAndNoFonts({super.key});

  @override
  State<FontChooserDropdownMenuWithoutServerAccessAndNoFonts> createState() =>
      _FontChooserDropdownMenuWithoutServerAccessAndNoFontsState();
}

class _FontChooserDropdownMenuWithoutServerAccessAndNoFontsState
    extends State<FontChooserDropdownMenuWithoutServerAccessAndNoFonts> {
  //final Map<SearchResponseEntryComparable, DropdownItem<SearchResponseEntryComparable>>
  final Map<String, DropdownItem<String>> _items = {};

  void _loadItems() {
    /*Set<SearchResponseEntryComparable> oldItems = _items.keys.toSet();
    bool changed = false;
    for (var value in controller.allFontsIfLoaded) {
      SearchResponseEntryComparable comparable =
      SearchResponseEntryComparable(value);
      if (!oldItems.contains(comparable)) {
        changed = true;
        _items[comparable] = DropdownItem<SearchResponseEntryComparable>(
          value: comparable,
          child: DropdownRowWidget(
            key: Key("${value.family}-dropdown-row-widget"),
            selectedFont: selectedFont,
            thisFont: value,
          ),
        );
      }
      oldItems.remove(comparable);
    }
    for (var item in oldItems) {
      _items.remove(item);
    }
    if (changed && mounted) {
        setState(() {});
    }*/
    for (int i = 0; i < 1500; i++) {
      _items["Font $i"] = DropdownItem<String>(
        value: "Font $i",
        child: DropdownRowWidget(
          key: Key("Font $i-dropdown-row-widget"),
          selectedFont: selectedFont,
          thisFont: "Font $i",
        ),
      );
    }
    setState(() {});
  }

  @override
  void initState() {
    super.initState();
    ServicesBinding.instance.keyboard.addHandler(_onKey);
    Future.microtask(_loadItems);
  }

  /*@override
  void onController() {
    var s = searchResponseEntryComparableFromSearchResponseEntry(
        controller.selectedFont?.searchResponseEntry);
    if (s != selectedFont.value) {
      selectedFont.value = s;
    }
    _loadItems();
  }*/

  @override
  void dispose() {
    ServicesBinding.instance.keyboard.removeHandler(_onKey);
    super.dispose();
  }

  //final ValueNotifier<SearchResponseEntryComparable?> selectedFont =
  final ValueNotifier<String?> selectedFont = ValueNotifier(null);

  TextEditingController searchController = TextEditingController();

  FocusNode searchFocusNode = FocusNode();

  bool _onKey(KeyEvent event) {
    /*setState(() {
      //set random height
      maxHeight = 200 + (100 * (DateTime.now().second % 5));
    });*/
    if (_menuOpen && !searchFocusNode.hasFocus && event is KeyDownEvent) {
      String? key = event.character;
      if (key != null &&
          event.logicalKey != LogicalKeyboardKey.enter &&
          event.logicalKey != LogicalKeyboardKey.space &&
          event.logicalKey != LogicalKeyboardKey.tab) {
        if (event.logicalKey == LogicalKeyboardKey.backspace) {
          if (searchController.text.isNotEmpty) {
            searchController.text = "";
            searchFocusNode.requestFocus();
            return true;
          }
          searchFocusNode.requestFocus();
          return false;
        }
        searchController.text += key;
        searchFocusNode.requestFocus();
        return true;
      }
    }
    return false;
  }

  bool _menuOpen = false;
  double? maxHeight;

  @override
  Widget build(BuildContext context) {
    //return DropdownButton2<SearchResponseEntryComparable>(
    return Expanded(child:Column(
      children: [
        DropdownButton2<String>(
          valueListenable: selectedFont,
          barrierCoversButton: false,

          //////////////////////////////////////////////////////////////////////
          /////IMPORTANT PART///////////////////////////////////////////////////
          //////////////////////////////////////////////////////////////////////
          dropdownOnlyBelowButton: false,//<-true for new option
          dropdownStyleData: DropdownStyleData(
            decoration: BoxDecoration(
              borderRadius: BorderRadius.circular(15),
            ),
            maxHeight: maxHeight,//<-comment out for new option
          ),
          //////////////////////////////////////////////////////////////////////

          onMenuStateChange: (open) {
            _menuOpen = open;
          },
          buttonStyleData: const ButtonStyleData(
            width: 260,
            height: 50,
          ),
          dropdownSearchData: DropdownSearchData(
            searchController: searchController,
            searchBarWidgetHeight: 50,
            searchBarWidget: Container(
              height: 50,
              padding: const EdgeInsets.only(
                top: 8,
                bottom: 4,
                right: 8,
                left: 8,
              ),
              child: TextFormField(
                maxLines: 1,
                controller: searchController,
                focusNode: searchFocusNode,
                decoration: InputDecoration(
                  isDense: true,
                  contentPadding: const EdgeInsets.symmetric(
                    horizontal: 10,
                    vertical: 8,
                  ),
                  hintText: 'Search for a font...',
                  hintStyle: const TextStyle(fontSize: 12),
                  border: OutlineInputBorder(
                    borderRadius: BorderRadius.circular(8),
                  ),
                ),
              ),
            ),
            noResultsWidget: const Padding(
              padding: EdgeInsets.all(8),
              child: Text('No Font Found!'),
            ),
            searchMatchFn:
                //(DropdownItem<SearchResponseEntryComparable> entry, String search) {
                (DropdownItem<String> entry, String search) {
              //return entry.value?.family
              return entry.value
                      ?.toLowerCase()
                      .contains(search.toLowerCase()) ??
                  false;
            },
          ),
          iconStyleData: const IconStyleData(
            icon: Icon(
              Icons.arrow_drop_down,
              color: Colors.black45,
            ),
          ),
          menuItemStyleData: const MenuItemStyleData(
            padding: EdgeInsets.symmetric(horizontal: 16),
          ),
          hint: const Text(
            'Select a font',
            style: TextStyle(fontSize: 14),
          ),
          /*onChanged: (SearchResponseEntryComparable? newValue) {
        selectedFont.value = newValue;
        if (newValue == null) {
          controller.selectedFont = null;
          return;
        }
        Future.microtask(() async {
          controller.selectedFont =
          await LoadedFont.fromSearchResponseEntry(newValue.entry);
        });
      },*/
          onChanged: (String? newValue) {
            selectedFont.value = newValue;
          },
          items: _items.values.toList(),
        ),
        
        ////////////////////////////////////////////////////////////////////////
        //ALSO IMPORTANT: HOW HEIGHT IS CALCULATED - MAYBE BUG HERE?////////////
        ////////////////////////////////////////////////////////////////////////
        Expanded(child: LayoutBuilder(
          builder: (context, constraints) {
            double h=constraints.maxHeight;
            if(h!=maxHeight){
              maxHeight=h;
              if(mounted){
                Future.microtask(() {
                  setState(() {});
                });
              }
            }
            return Container();
          },
        )),
        ////////////////////////////////////////////////////////////////////////
      ],
    ),);
  }
}

class DropdownRowWidget extends StatefulWidget {
  //final SearchResponseEntry thisFont;
  final String thisFont;

  //final ValueNotifier<SearchResponseEntryComparable?> selectedFont;
  final ValueNotifier<String?> selectedFont;
  final double width;

  const DropdownRowWidget(
      {super.key,
      required this.thisFont,
      this.width = 200,
      required this.selectedFont});

  @override
  State<DropdownRowWidget> createState() => _DropdownRowWidgetState();
}

class _DropdownRowWidgetState extends State<DropdownRowWidget> {
  bool _isFontLoaded = false;
  bool _isLoading = false;

  @override
  Widget build(BuildContext context) {
    return Row(
      mainAxisAlignment: MainAxisAlignment.spaceBetween,
      children: [
        SizedBox(
          width: widget.width,
          child: Text(
            widget.thisFont,
            /*style: Theme.of(context)
                  .textTheme
                  .apply(fontFamily: widget.thisFont.menuFontFamily)
                  .bodyMedium*/
          ),
        ),
        if (!_isFontLoaded && !_isLoading)
          const Icon(
            Icons.cloud_download_outlined,
            color: Colors.grey,
            size: 24,
          ),
        /*if (!_isFontLoaded && _isLoading)
          LoadingAnimationWidget.dotsTriangle(
            color: Colors.grey,
            size: 24,
          ),*/
      ],
    );
  }

  void _check() {
    if (!mounted) {
      return;
    }
    bool s = false;
    //bool i = isFontLoaded(widget.thisFont);
    bool i = false;
    if (i != _isFontLoaded) {
      _isFontLoaded = i;
      s = true;
    }
    //i = isLoadingFont(widget.thisFont);
    i = false;
    if (i != _isLoading) {
      _isLoading = i;
      s = true;
    }
    if (s) {
      setState(() {});
    }
  }

  @override
  void initState() {
    super.initState();
    _check();
    //FontLoaderNotifier.instance.addListener(_check);
    widget.selectedFont.addListener(_check);
  }

  @override
  void didUpdateWidget(DropdownRowWidget oldWidget) {
    super.didUpdateWidget(oldWidget);
    _check();
  }

  @override
  void dispose() {
    //FontLoaderNotifier.instance.removeListener(_check);
    super.dispose();
  }
}

@hlvs-apps
Copy link
Contributor Author

A thought to improve UX with the new proposed option enabled: We could check if the space forced with the option enabled is enough to display the menu (with maybe 1 to 2 rows visible at least) and if not fall back to the default behaviour (option disabled / non existent)

@hlvs-apps
Copy link
Contributor Author

hlvs-apps commented May 30, 2024

On the other side, I get it that this PR would complicate testing, and overall constantly performing a good and understandable experience for those who don't want to read the source code of every lib they use.

What do you think of introducing a new callback function to calculate the available height? This way, this package could allow this special use case (and potentionally others), while taking away the responsibility for a good UX in this case from the package.

The callback function could be defined as:

double? Function(double, EdgeInsets,Rect)? calculateMenuAvailableHeight;

and could be used in the 'getMenuAvailableHeight' function in dropdown_route.dart like:

  double getMenuAvailableHeight(
    double availableHeight,
    EdgeInsets mediaQueryPadding,
    Rect buttonRect,
  ) {
    double? callbackHeight = calculateMenuAvailableHeight != null
        ? calculateMenuAvailableHeight(
            availableHeight, mediaQueryPadding, buttonRect)
        : null;
    return math.max(
      0.0,
      callbackHeight ??
          (availableHeight - mediaQueryPadding.vertical - _kMenuItemHeight),
    );
  }

@AhmedLSayed9
Copy link
Owner

But if the available space and therefore the calculated max-height changes, e.g. because the windows is resized, the changes made to the max height property only take effect after the dropdown menu is reopened

It's desired that the dropdown can go above the button when there isn't enough space underneath and I think that's the correct behavior.

Forcing the menu to always open beneath the button will lead to issues that's hard to catch and bad UX. i.e:

Screen.Recording.2024-06-01.at.7.04.14.PM.mov
Screen.Recording.2024-06-01.at.7.07.05.PM.mov
Screen.Recording.2024-06-01.at.7.09.33.PM.mov

@hlvs-apps
Copy link
Contributor Author

It's desired that the dropdown can go above the button when there isn't enough space underneath and I think that's the correct behavior.

I agree that the dropdown should go above the button when there isnt't enough space underneath, the problem I have is that the current implementation always decides that there isn't enough space, even if there is enough space to display the menu with multiple rows and the search widget underneath and going above the button only gains 1-2 rows.

Issues like in this video

Screen.Recording.2024-06-01.at.7.04.14.PM.mov

can also happen if the developer sets a very low maximum height - following your line of argumentation we should add checks to make sure the maximum height is big enough to display the dropdown menu and ignore/increase this maximum height passed by the app developer if it is to low for an 'good' UX - or we should remove it for simplicit to enforce an 'good' UX.

IMO there is no reason to do this - after all this project is just a tool used by many apps, and the responsibility for a good UX ultimately lies with the app developer. Of course this project should encourage good UX and support the app developer to implement it, but giving the app developer more freedom to change the look of this package in a way that also enables bad UX doesn't necessarily mean bad UX in the app, no it also enables better UX for certain use cases.

I get it that my proposed parameter 'dropdownOnlyBelowButton' may indeed allow bad UX if not understood correctly by the app developer, but this doesn't mean the app developer should not have this option in a way that decreases the likelihood of a bad UX.

What do you think of introducing a new callback function to calculate the available height? This way, this package could allow this special use case (and potentionally others), while taking away the responsibility for a good UX in this case from the package.

The callback function could be defined as:

double? Function(double, EdgeInsets,Rect)? calculateMenuAvailableHeight;

and could be used in the 'getMenuAvailableHeight' function in dropdown_route.dart like:

  double getMenuAvailableHeight(
    double availableHeight,
    EdgeInsets mediaQueryPadding,
    Rect buttonRect,
  ) {
    double? callbackHeight = calculateMenuAvailableHeight != null
        ? calculateMenuAvailableHeight(
            availableHeight, mediaQueryPadding, buttonRect)
        : null;
    return math.max(
      0.0,
      callbackHeight ??
          (availableHeight - mediaQueryPadding.vertical - _kMenuItemHeight),
    );
  }

If the app developer takes the risk and implements his own 'calculateMenuAvailableHeight' I think this package is not responsible for the negative effects caused by the app developers' implementation - just like this package is not responsible for an inapropiate max height that leads to bad UX.

I would be happy to create a new PR for this, just dont't want to waste that time.

@AhmedLSayed9
Copy link
Owner

AhmedLSayed9 commented Jun 1, 2024

the problem I have is that the current implementation always decides that there isn't enough space, even if there is enough space to display the menu with multiple rows and the search widget underneath and going above the button only gains 1-2 rows.

The current implementation decides if there isn't enough space as expected.

If you set max height to 300, and there's enough space to be displayed it'll show beneath the button. Otherwise, it'll shift up to display the expected menu height.

I think this pr is not about opening below button but more about "dynamically resizing the menu" which I'm skeptical about it for the mentioned issues and for the fact that I don't find a real use case for it, i.e:

  • If the button is sticky at the top of the screen (your use case), we can just set reasonable max height for the menu. If the user minimize the window? imo, in that case it's better to show him more options above the button than just show 1 or 2 options, bad ux and possible overflow exceptions.
  • If the button is not sticky at the top, a lot of issues might occur as mentioned above.

can also happen if the developer sets a very low maximum height

No, this is a different thing. This will be a decision by the user that won't work fine at first place.

The proposed option would work fine, but i.e if the user minimize the window or when scrolling the button to the bottom, it'll lead to issues like overflow or the user will press the button and menu won't show at all.

@AhmedLSayed9
Copy link
Owner

imo, a better proposal that will avoid the mentioned issues for the developer/user experience, is a minHeight property similar to maxHeight. It'll be optional and if it's set to say 200px, then the dropdown menu can be minimized when there isn't enough space but not lower than 200px.

I'll need some time to think more about it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants